Skip to content
This repository has been archived by the owner on Aug 31, 2023. It is now read-only.

feat(rome_js_analyzer): noSelfAssignment rule #4091

Merged
merged 10 commits into from
Feb 14, 2023
Merged

Conversation

ematipico
Copy link
Contributor

@ematipico ematipico commented Dec 22, 2022

Summary

Part of #3980

This PR starts the implementation of the noSelfAssignment rule. The rule is quite lengthy so I had to stop at sont toless too much meat to the fire.

The implementation of this is quite different from the ESLint one. The ESLint implementation is "simple" but has various pitfalls:

  • it relies on recursion
  • there's a double loop with recursion

To avoid this memory pressure I had to come up with something, so implementation relies on:

  • one single loop
  • iterators
  • a queue

The gist of the implementation is the following:

  • a JsAssignmentExpression is broken in two arms: left-arm and right-arm, which is called AnyJsAssignmentLike
  • creates an ad-hoc iterator; this iterator emits similar pairs of "identifiers". If these identifiers are not identical, the loop continues until the iterator is consumed . For example, in a code a = a, it returns the pair of (a, a), but in a code like [a, 1, b] = [a, c, b], the first iteration returns (a, a) but the second iteration return None because 1 and c are not similar (their AST nodes are different), and the third returns (b,b)
  • each pair gets checked, and if they have the exact text, they are tracked and then emitted in the diagnostic

The ad-hoc iterator works this way:

  • the two arms (left and right) are transformed into two iterators;
  • every time we need to compute a new pair of identifiers, we call .next() on both iterators; both items need to exist and need to be "similar" (there are various matches to check if they are similar), failing to adhere to these two requirements will continue to the next item
  • every time we find a "substructure" (e.g. object assignment pattern inside an array assignment pattern, for example: [{a, b}, c] = [{a, b}, c], we stop the current iteration, save it into a queue, and we start a new one. We resume the previous one when we "drain" this latest iteration.

This implementation doesn't cover:

  • parenthesis
  • optional chain
  • string literals inside a computed member expression, e.g. a['b']

Test Plan

The tests are taken from the test suite of ESLint

Documentation

  • The PR requires documentation
  • I will create a new PR to update the documentation

@ematipico ematipico requested review from leops, xunilrj and a team as code owners December 22, 2022 16:29
@netlify
Copy link

netlify bot commented Dec 22, 2022

Deploy Preview for docs-rometools ready!

Name Link
🔨 Latest commit 9e621b9
🔍 Latest deploy log https://app.netlify.com/sites/docs-rometools/deploys/63eb6146b07e2d0007f92f4c
😎 Deploy Preview https://deploy-preview-4091--docs-rometools.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@github-actions
Copy link

This PR is stale because it has been open 14 days with no activity.

@ematipico ematipico added this pull request to the merge queue Feb 14, 2023
Merged via the queue into main with commit 149e800 Feb 14, 2023
@ematipico ematipico deleted the feat/no-self-assign branch February 14, 2023 10:24
@ematipico ematipico removed the S-Stale label Feb 14, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant